Skip to content

SIP-NN - Curried varargs #1487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 26, 2019
Merged

SIP-NN - Curried varargs #1487

merged 3 commits into from
Aug 26, 2019

Conversation

Atry
Copy link
Contributor

@Atry Atry commented Aug 11, 2019

@lihaoyi
Copy link
Contributor

lihaoyi commented Aug 25, 2019

I like this a lot:

  • It is a natural solution to the “should varargs be mutable or inmutable or readonly Seq” question: with a builder, you can make your vararg take exactly the type you need, even unusual ones like Map or Set or scalatags.text.Builder, and have it be constructed directly without overhead

  • It provides more information to the compiler and optimizer: where the number of args at a callsite is fixed, simple inlining produces a fixed set of instructions to run on a builder, which makes further optimization possible. The current Seq-based model obfuscates everything by forcing the params into a variable length homogenous sequence of boxes, which any optimizer would have to deobfuscate to figure out is fixed length and homogenous

  • The fact that the builder’s method calls can be overloaded can remove yet another layer of boxing when taking heterogenous varargs: currently we have to take varargs of a supertype/trait and have the possible parameter types boxed into compliance with the vararg type via implicits

  • It has plenty of precedence: it’s just the Visitor Pattern or Finally Tagless. Like how uPickle uses visitors to abstract over multiple ASTs and JSON libraries with a performance boost instead of a penalty, or SAX parsers allow efficient streaming XML transforms unlike DOM based approaches. This would let varargs abstract over multiple possible collection types, and even non collection types, also with a performance boost

  • The fact that it could allow a heterogenous set of types to be inferred from a variadic argument list opens up a lot of possibilities, e.g. it makes translation of Tuple.apply(x, y, z) calls into HList-esque tuples trivial to implement in userland, a nice Rec(“foo” -> bar) syntax for record-types also entirely in userland, and probably other things that I can’t come up with off the top of my head

  • I think this would allow the current awkward builder-based applicative zipMap syntax (foo |@| bar |@| baz)(f) to be written more ergonomically as zipMap(foo, bar, baz)(f) while still being variadic

These benefits are all on top of those that Yang Bo lists, e.g. better perf for XML literal construction, string interpolators, and collection companion factories.

It’s not common that a new technique allows both better abstraction, better flexibility, and better performance. This definitely seems like one of those cases. Details will need to be worked out, but in principle this is great!

@valenterry
Copy link
Contributor

  • The fact that it could allow a heterogenous set of types to be inferred from a variadic argument list opens up a lot of possibilities, e.g. it makes translation of Tuple.apply(x, y, z) calls into HList-esque tuples trivial to implement in userland, a nice Rec(“foo” -> bar) syntax for record-types also entirely in userland, and probably other things that I can’t come up with off the top of my head

I agree! This would be huge for making dependently typed apis much more user friendy.

Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the proposal, @Atry. We'll discuss it at the next SIP meeting. There is not scheduled date yet.

@sjrd sjrd merged commit 320fb0e into scala:master Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants